Skip to content

fix: trim whitespace from API keys and host config - #181

Merged
marandaneto merged 6 commits into
mainfrom
fix/trim-whitespace-in-config-keys-and-host
Apr 21, 2026
Merged

fix: trim whitespace from API keys and host config#181
marandaneto merged 6 commits into
mainfrom
fix/trim-whitespace-in-config-keys-and-host

Conversation

@marandaneto

@marandaneto marandaneto commented Apr 20, 2026

Copy link
Copy Markdown
Member

💡 Motivation and Context

This ports the whitespace-trimming fix from posthog-go to posthog-dotnet. It trims surrounding spaces and line breaks from the project API key and personal API key before using them so the SDK does not silently send invalid tokens or treat whitespace-only personal API keys as valid local-evaluation credentials.

💚 How did you test it?

  • dotnet test tests/UnitTests/UnitTests.csproj -f net8.0 --filter "FullyQualifiedName~LogsWarningWhenPersonalApiKeyIsBlankAfterTrimmingWhitespace|FullyQualifiedName~LogsErrorWhenProjectApiKeyIsBlankAfterTrimmingWhitespace|FullyQualifiedName~LogsWarningWhenPersonalApiKeyIsNull"

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Added the release label to the PR
  • Added exactly one version bump label: bump-patch, bump-minor, or bump-major

@marandaneto
marandaneto requested a review from haacked as a code owner April 20, 2026 13:11
@marandaneto
marandaneto requested a review from a team as a code owner April 20, 2026 13:11
@github-actions

github-actions Bot commented Apr 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

posthog-dotnet Compliance Report

Date: 2026-04-21 11:59:27 UTC
Duration: 264ms

⚠️ Some Tests Failed

0/1 tests passed, 1 failed

Feature_Flags Tests

⚠️ 0/1 tests passed, 1 failed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ❌ 44ms

Failures

request_payload.request_with_person_properties_device_id

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag'

@greptile-apps

greptile-apps Bot commented Apr 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Comments Outside Diff (2)

  1. src/PostHog/PostHogClient.cs, line 857-860 (link)

    P1 Duplicate EventId 18 introduced by renumbering

    The PR renumbered LogErrorFailedToLoadFeatureFlags from 17 → 18, but LogErrorCaptureExceptionNull already used EventId 18 and was not renumbered. Both methods now share EventId 18, meaning log filtering and correlation by event ID will be broken for both messages.

    Prompt To Fix With AI 
    This is a comment left during a code review.
Path: src/PostHog/PostHogClient.cs
Line: 857-860

Comment:
**Duplicate EventId 18 introduced by renumbering**

The PR renumbered `LogErrorFailedToLoadFeatureFlags` from 17 → 18, but `LogErrorCaptureExceptionNull` already used EventId 18 and was not renumbered. Both methods now share EventId 18, meaning log filtering and correlation by event ID will be broken for both messages.



How can I resolve this? If you propose a fix, please make it concise.

  2. tests/UnitTests/PostHogClientTests.cs, line 1427-1452 (link)

    P2 Prefer parameterised tests per team convention

    LogsWarningWhenPersonalApiKeyIsNull and LogsWarningWhenPersonalApiKeyIsBlankAfterTrimmingWhitespace exercise the exact same code path with two different inputs (null vs whitespace-only). Per the team's convention these should be consolidated into a single [Theory]/[InlineData] test rather than two separate [Fact] tests.

    Context Used: Do not attempt to comment on incorrect alphabetica... (source)

    Prompt To Fix With AI 
    This is a comment left during a code review.
Path: tests/UnitTests/PostHogClientTests.cs
Line: 1427-1452

Comment:
**Prefer parameterised tests per team convention**

`LogsWarningWhenPersonalApiKeyIsNull` and `LogsWarningWhenPersonalApiKeyIsBlankAfterTrimmingWhitespace` exercise the exact same code path with two different inputs (null vs whitespace-only). Per the team's convention these should be consolidated into a single `[Theory]/[InlineData]` test rather than two separate `[Fact]` tests.

**Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

How can I resolve this? If you propose a fix, please make it concise.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix All With AI 
This is a comment left during a code review.
Path: src/PostHog/PostHogClient.cs
Line: 857-860

Comment:
**Duplicate EventId 18 introduced by renumbering**

The PR renumbered `LogErrorFailedToLoadFeatureFlags` from 17 → 18, but `LogErrorCaptureExceptionNull` already used EventId 18 and was not renumbered. Both methods now share EventId 18, meaning log filtering and correlation by event ID will be broken for both messages.

```suggestion
    [LoggerMessage(
        EventId = 19,
        Level = LogLevel.Error,
        Message = "CaptureException called with null exception")]
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: tests/UnitTests/PostHogClientTests.cs
Line: 1427-1452

Comment:
**Prefer parameterised tests per team convention**

`LogsWarningWhenPersonalApiKeyIsNull` and `LogsWarningWhenPersonalApiKeyIsBlankAfterTrimmingWhitespace` exercise the exact same code path with two different inputs (null vs whitespace-only). Per the team's convention these should be consolidated into a single `[Theory]/[InlineData]` test rather than two separate `[Fact]` tests.

**Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix: trim whitespace from API keys and h..." | Re-trigger Greptile

dustinbyrne
dustinbyrne approved these changes Apr 20, 2026
View reviewed changes

@dustinbyrne dustinbyrne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving to unblock, but the null check is needed

Comment thread src/PostHog/PostHogClient.cs Outdated
haacked
haacked reviewed Apr 20, 2026
View reviewed changes
Comment thread src/PostHog/PostHogClient.cs Outdated
@marandaneto
marandaneto merged commit e9cbbfa into main Apr 21, 2026
15 checks passed
@marandaneto
marandaneto deleted the fix/trim-whitespace-in-config-keys-and-host branch April 21, 2026 12:02
@dependabot dependabot Bot mentioned this pull request Apr 25, 2026
Closed
This was referenced May 2, 2026
Closed
Closed
@dependabot dependabot Bot mentioned this pull request May 16, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@haacked haacked haacked left review comments
@dustinbyrne dustinbyrne dustinbyrne approved these changes

Assignees

No one assigned

Labels

bump-patch release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@marandaneto @haacked @dustinbyrne